[backport camel-4.18.x] CAMEL-26322: camel-as2 - scope the async MDN allow-list to host and port, and add an option to require https by gnodet · Pull Request #29055 · apache/camel

gnodet · 2026-11-03T08:51:53Z

Backport of #28527

Cherry-pick of 8cedca3 to camel-4.18.x.
Conflicts resolved automatically by backport bot agent.

…allow-list to host and port, and add an option to require https

gnodet · 2026-11-03T09:04:46Z

The backport is incomplete: the cherry-pick carried requireHttps configuration wiring from main but omitted the underlying HTTPS delivery implementation (SSLContext support, scheme-branching in send()). The result is dead security code and tests that won't compile.

🔴 Compile-time failures

Finding 3 — Test constructors called with 11 args, but constructor has 10 parameters

AS2ServerConnectionTest.aBareHostEntryMatchesTheDefaultPort (and theWarningNamesTheAuthorityAsAnAllowListEntry) pass 11 arguments to AS2ServerConnection:

new AS2ServerConnection("1.1", "Camel", "sender.example.com",
    null, null, "user", "password", null, ALLOWED, null, false)

But the constructor added by this PR has exactly 10 parameters (as2Version, userAgent, senderFQDN, signingCertificateChain, signingPrivateKey, userName, password, accessToken, allowedHosts, requireHttps). The extra null (intended for an SSLContext that exists on main but was not backported) has no matching parameter. Will not compile.

Finding 4 — Tests call a 4-arg deliver() overload that doesn't exist

httpIsRefusedWhenHttpsIsRequired calls deliver("http://partner.example/receipts", ALLOWED, null, true) and httpsIsAcceptedWhenHttpsIsRequired calls deliver("https://localhost:1/receipts", "localhost:1", sslContext, true) — both with 4 arguments. The PR only adds 2-arg and 3-arg overloads; the 4-arg deliver(String, String, SSLContext, boolean) was not backported. Will not compile.

🔴 Dead security control

Finding 1 — requireHttps field assigned but never read in send()

requireHttps is declared as a field and assigned in the constructor, but the send() method in AS2AsynchronousMDNManager never checks it. The field is a dead store — there is no if (requireHttps && !url.startsWith("https://")) guard or equivalent. Operators who set asyncMdnRequireHttps=true believing they are enforcing HTTPS will get no enforcement.

Finding 2 — requireHttps wiring present end-to-end, enforcement missing

The configuration plumbing is complete (AS2Configuration, AS2ConsumerConfiguration, endpoint DSL, generated configurers) but the enforcement point is absent. This is the root cause of Finding 1: the feature was not fully backported.

Finding 5 — Documentation describes behaviour that doesn't exist

The updated documentation says "a bare host matches only the default port for the scheme (80 for http, 443 for https)", but send() hardcodes int defaultPort = 80 with no scheme-branching logic. The 443 case is simply not implemented.

Finding 6 — Javadoc on getter/setter promises enforcement that doesn't exist

AS2Configuration.setAsyncMdnRequireHttps() Javadoc states "an asynchronous MDN whose delivery address does not use https is refused". It is not refused. This creates a false sense of security for users relying on the documentation.

What's missing from the backport

The following pieces exist on main but were not cherry-picked:

AS2AsynchronousMDNManager constructor accepting SSLContext
AS2ServerConnection 11-parameter constructor accepting SSLContext
createSocket(SSLContext) / scheme-branching in send()
4-arg deliver(String, String, SSLContext, boolean) test helper
The compile-time guard that makes requireHttps=true actually refuse non-HTTPS URLs

Until these are included, requireHttps is inert and the tests will not build.

davsclaus

Thanks for picking this up. The allow-list now matching host and port is a useful change for 4.18, but this port does not build yet, and part of it depends on a change that is not on this branch:

Compile error. AS2ServerConnection gets this.sslContext = sslContext; (around line 676), but 4.18 has no sslContext field on that class, hence cannot find symbol in CI.
asyncMdnRequireHttps needs TLS delivery, which 4.18 does not have. On main, https delivery of asynchronous MDNs came with CAMEL-24474 (4.23). On 4.18, AS2AsynchronousMDNManager.send() still refuses every non-http address ("TLS delivery of asynchronous MDNs is not supported"). With this port, requireHttps is stored but never read; if it were enforced, it would refuse every delivery, since http is the only scheme that works here. The option's Javadoc and docs promise behaviour the branch cannot provide, and the httpsIsAcceptedWhenHttpsIsRequired/httpIsRefusedWhenHttpsIsRequired tests rely on an SSLContext overload of deliver. The bare-host text "443 for https" in as2-component.adoc does not apply on 4.18 either.

I'd suggest limiting the 4.18 port to the host-and-port scoping of asyncMdnAllowedHosts (with isAllowedAuthority, its tests and the doc text about host:port / IPv6 brackets), and leaving asyncMdnRequireHttps out. What do you think?
Upgrade note. A bare-host entry now matches only the default port, so deployments receiving MDNs on another port have to add the port. That needs an entry under == Upgrading from 4.18.5 to 4.18.6 in camel-4x-upgrade-guide-4_18.adoc, on this branch and on main (main only has it in the 4_24 guide).

Reviewed with Claude Code on behalf of Claus Ibsen (@davsclaus). This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

…in backport

…o SSLContext)

gnodet-bot

Following up on my earlier review — all 6 previously raised issues have been addressed. The port-aware allow-list and requireHttps enforcement are correctly implemented. Two documentation inaccuracies remain.

✅ Previous findings resolved

Finding 1 — requireHttps field was dead (never read in send()): ✅ fixed — the check if (requireHttps && !"https".equals(scheme)) is now present.
Finding 2 — wiring complete but enforcement missing: ✅ fixed — end-to-end plumbing (AS2Configuration → AS2ConsumerConfiguration → AS2ConnectionHelper → AS2Consumer → AS2AsynchronousMDNManager) is complete.
Finding 3 — test constructors called with 11 args (SSLContext not backported): ✅ fixed — 10-parameter form used.
Finding 4 — tests calling 4-arg deliver() that didn't exist: ✅ fixed — 3-arg deliver(address, allowedHosts, requireHttps) overload added.
Finding 5 — defaultPort hardcoded to 80 with no scheme-branching: ✅ acknowledged — the implementation only supports http (see remaining issues below for the doc side).
Finding 6 — Javadoc promised enforcement that didn't exist: ✅ fixed — Javadoc on setAsyncMdnRequireHttps accurately describes behaviour that now exists.

The isAllowedAuthority / authorityMatches logic is correct: bracketed IPv6 parsing, bare-host → default-port, and host:port entries all handled properly.

⚠️ Two documentation inaccuracies

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…dnRequireHttps on camel-4.18.x - asyncMdnAllowedHosts: bare-host entries match only port 80 (TLS not supported); remove '443 for https' from Javadoc, user docs and catalog JSON - asyncMdnRequireHttps: clarify that only http is accepted in this branch; TLS delivery of async MDNs is not yet implemented - Add upgrade guide entry for the host-and-port matching behavior change

gnodet-bot

Following up on my earlier review — both previously raised documentation inaccuracies have been addressed in AS2Configuration.java and as2-component.adoc. However, the generated artifacts were regenerated with inconsistent Javadoc text and still contain the old inaccuracies.

✅ Previous findings resolved

Finding 1 — asyncMdnAllowedHosts Javadoc claimed "80 for http, 443 for https" default ports: ✅ fixed in AS2Configuration.java — now correctly states "matches only the default port (80)".
Finding 2 — asyncMdnRequireHttps Javadoc stated "by default both http and https are accepted": ✅ fixed in AS2Configuration.java — now correctly states "by default only http is accepted (TLS delivery of asynchronous MDNs is not yet implemented)".
Finding 3 (from original davsclaus review) — upgrade guide entry: ✅ added under camel-4x-upgrade-guide-4_18.adoc.

⚠️ Two generated artifacts still carry the stale descriptions

This review was generated by an AI agent, Hermès on behalf of @gnodet.

…oc fixes

gnodet-bot

Following up on my earlier review — all previous findings are resolved.

✅ davsclaus concerns resolved

Compile error (sslContext field): ✅ fixed — sslContext is now passed as a constructor parameter, not stored as a field. No this.sslContext = sslContext assignment in the diff.
asyncMdnRequireHttps in 4.18: ✅ addressed — the option now enforces its security guarantee (credentials are never sent over a sender-chosen http address) with honest Javadoc: "by default only http is accepted (TLS delivery of asynchronous MDNs is not yet implemented)." Setting it to true is a valid opt-in posture for strict operators.
Upgrade note: ✅ added under "Upgrading from 4.18.5 to 4.18.6" in camel-4x-upgrade-guide-4_18.adoc.

One nit

One Javadoc line runs to 161 characters. The inline comment has a suggestion.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

CI's regen.sh detected comment wrap differences in AS2ServerConnection.java, AS2Configuration.java, and the generated AS2EndpointBuilderFactory.java. Apply the exact rewrapping the code generator produces to eliminate the "uncommitted changes" CI failure.

gnodet-bot

Following up on my earlier review — the nit (161-char Javadoc line) from the last pass is resolved.

✅ All previous findings resolved

Port-aware allow-list: ✅ implemented — isAllowedAuthority() + authorityMatches() match the full host:port authority.
requireHttps wiring: ✅ complete — field added to AS2ConsumerConfiguration, propagated through AS2ServerConnection and wired into AS2AsynchronousMDNManager.
Test coverage: ✅ AS2ServerTwoConsumerAsyncMdnSettingsTest updated to use localhost:PORT entries; new unit tests for port-based matching and requireHttps added.
Upgrade guide: ✅ documents the bare-host → default-port change.

One remaining gap — upgrade guide omits IPv6 format change

The upgrade guide correctly documents the non-default port migration (partner.example → partner.example:8443), but omits a second breaking change that affects IPv6 users.

Before this PR: isAllowedHost() compared entry.trim().equalsIgnoreCase(host) — host-only, no port. normalizeHost() strips brackets from URI.getHost(), so host = "::1". An entry ::1 (unbracketed) matched delivery to http://[::1]:1/ regardless of port.

After this PR: authorityMatches("::1", "::1", 1, 80) — entry ::1 has multiple colons → falls into the "unbracketed IPv6 → bare-host" branch → entryPort = 80, entryPort (80) ≠ port (1) → refused. The same applies even at port 80: an entry ::1 now requires [::1]:80 to match http://[::1]:80/.

A deployment with asyncMdnAllowedHosts=::1 that worked before this commit silently stops matching all IPv6 MDN deliveries. The inline comment has a suggestion.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

gnodet-bot

Following up on my earlier review — all 6 findings have been addressed in this update.

Finding 1 (requireHttps dead store) — ✅ Addressed. send() now enforces the flag with if (requireHttps && !"https".equals(scheme)) throw.

Finding 2 (requireHttps wiring without enforcement) — ✅ Addressed. End-to-end plumbing from AS2Configuration → AS2ConsumerConfiguration → AS2AsynchronousMDNManager now carries the flag through to enforcement.

Finding 3 (constructor arg-count mismatch) — ✅ Addressed. Overloaded constructors added to AS2AsynchronousMDNManager and AS2ServerConnection; the original constructors now delegate to the new ones with requireHttps=false, preserving backward compatibility.

Finding 4 (missing 4-arg deliver() test helper) — ✅ Addressed. A 3-arg deliver(address, hosts, requireHttps) overload is added; the 4-arg variant with SSLContext is appropriately out of scope for the 4.18.x backport where TLS delivery is not yet implemented.

Finding 5 (defaultPort=80 with no scheme-awareness) — ✅ Addressed (scope). On camel-4.18.x, the existing scheme check (if (!"http".equals(scheme))) already rejects https, so defaultPort=80 is correct for this branch. The isAllowedAuthority(host, port, defaultPort) API is forward-compatible — when TLS delivery lands, callers pass defaultPort=443 for https.

Finding 6 (Javadoc promises enforcement that doesn't exist) — ✅ Addressed. The enforcement is now real; the Javadoc is accurate.

The upgrade guide entry correctly documents the breaking change to asyncMdnAllowedHosts (now matches host+port, not host only), and the tests for asyncMdnRequireHttps (httpIsRefusedWhenHttpsIsRequired, httpIsAcceptedWhenHttpsIsNotRequired) exercise the new enforcement path.

LGTM.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[backport camel-4.18.x] CAMEL-26322: camel-as2 - scope the async MDN …

9774433

…allow-list to host and port, and add an option to require https

gnodet added the backport indicate that a Pull request is a backport from a fix from the main branch label Nov 3, 2026

gnodet assigned davsclaus Nov 3, 2026

gnodet mentioned this pull request Nov 3, 2026

CAMEL-26322: camel-as2 - scope the async MDN allow-list to host and port, and add an option to require https #28527

Merged

davsclaus reviewed Nov 3, 2026

View reviewed changes

gnodet added 2 commits November 3, 2026 16:52

fix: remove spurious this.sslContext assignment — field not declared …

43de334

…in backport

fix: enforce requireHttps in send() and adapt test to backport API (n…

69a9b9b

…o SSLContext)